Skip to content

fix(orchestrator): scope Claude MCP tool pre-approval#3862

Open
PixPMusic wants to merge 2 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-claude-mcp-tools
Open

fix(orchestrator): scope Claude MCP tool pre-approval#3862
PixPMusic wants to merge 2 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-claude-mcp-tools

Conversation

@PixPMusic

@PixPMusic PixPMusic commented Jul 10, 2026 •
edited by macroscopeapp Bot
Loading

Copy link
Copy Markdown

Summary

  • always pre-approve the thread-scoped T3 MCP server's tools when it is attached: the Claude Agent SDK's allowedTools only pre-approves tool calls (availability is the separate tools option), so without this entry headless modes like dontAsk deny the orchestrator's own mcp__t3-code__* calls
  • in read-only sandboxes, pre-approve only the orchestrator tools annotated Tool.Readonly instead of the wildcard, so a read-only session cannot silently spawn child threads or scheduled tasks
  • cover MCP and allowlist combinations with focused adapter tests, including a cross-check that the read-only allowlist stays in sync with the toolkit annotations

Notes

An earlier revision of this PR instead dropped the wildcard when no explicit allowlist was set, on the premise that it blocked Claude's built-in tools. Per the SDK typings, allowedTools cannot block built-in tools ("To restrict which tools are available, use the tools option instead"), and dropping it would have auto-denied or prompt-gated the t3-code MCP tools in every mode that does not bypass permissions.

Validation

  • vp check
  • vp run typecheck
  • vp test apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts

Addresses review feedback on #2829.

Note

Scope Claude MCP tool pre-approval to read-only tools in read-only sandboxes

  • In read-only sandboxes, claudeMcpQueryOverrides now pre-approves only the fixed set of tools in CLAUDE_READ_ONLY_T3_MCP_ALLOWED_TOOLS instead of the mcp__t3-code__* wildcard.
  • Adds claudeEffectiveQueryPolicyKey to incorporate MCP-derived allowedTools into live-query reuse keys, so read-only and non-read-only queries are not incorrectly shared.
  • claudeMcpQueryOverrides now requires a readOnlySandbox boolean; callers in openQuery derive it from sandboxPolicyKindForClaudeRuntimePolicy.
  • Behavioral Change: read-only sandbox sessions no longer have access to all t3-code MCP tools; access is limited to the tools annotated with Tool.Readonly in OrchestratorToolkit.

Macroscope summarized 4eafde4.

Note

Medium Risk
Changes permission pre-approval for orchestrator MCP tools in read-only sandboxes; behavior is narrow and covered by focused adapter tests, but it affects what agents can do without prompts in constrained runtimes.

Overview
When a thread-scoped t3-code MCP server is attached, Claude query setup still pre-approves orchestrator MCP tools via allowedTools, but read-only sandboxes now get only CLAUDE_READ_ONLY_T3_MCP_ALLOWED_TOOLS instead of the mcp__t3-code__* wildcard, so headless modes cannot auto-approve thread spawn or scheduled-task mutations.

claudeMcpQueryOverrides takes a readOnlySandbox flag (derived from the runtime sandbox policy at query open). Live-query reuse uses claudeEffectiveQueryPolicyKey so differing MCP pre-approvals do not share a cached query. Tests cover MCP/no-MCP and allowlist combinations, plus a check that the read-only list stays aligned with Tool.Readonly on OrchestratorToolkit.

Reviewed by Cursor Bugbot for commit 4eafde4. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e1d89a73-d790-436a-b7bf-96ce7d333995

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Jul 10, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 10, 2026
View reviewed changes
@macroscopeapp

macroscopeapp Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR modifies security-relevant tool authorization logic, gating which MCP tools are pre-approved based on sandbox read-only status. Changes that affect authorization behavior and restrict capabilities in security contexts warrant human review despite good test coverage.

You can customize Macroscope's approvability policy. Learn more.

@PixPMusic
fix(orchestrator): scope Claude MCP tool pre-approval
744c7b7 
The Claude Agent SDK's allowedTools option only pre-approves tool calls;
tool availability is controlled by the separate tools option. Attaching
the t3-code MCP server therefore always pre-approves its tools so
headless permission modes (dontAsk) do not deny the orchestrator's own
MCP calls. Read-only sandboxes now pre-approve only the orchestrator
tools annotated read-only, so a read-only session can no longer silently
spawn child threads or scheduled tasks.
@PixPMusic PixPMusic force-pushed the pixpmusic/fix-claude-mcp-tools branch from 3ffc8e1 to 744c7b7 Compare July 10, 2026 19:29
@macroscopeapp macroscopeapp Bot dismissed their stale review July 10, 2026 19:29

Dismissing prior approval to re-evaluate 744c7b7

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Jul 10, 2026
@PixPMusic PixPMusic changed the title fix(orchestrator): preserve Claude built-in tools with MCP fix(orchestrator): scope Claude MCP tool pre-approval Jul 10, 2026
cursor[bot]
cursor Bot reviewed Jul 10, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 744c7b7. Configure here.

Comment thread apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts
@PixPMusic
fix(orchestrator): key query reuse on effective allowlist
4eafde4 
Reused live queries were keyed only by the runtime policy, so policies
sharing a policy key could reuse a query opened with different MCP
pre-approvals (wildcard vs read-only list) or without the MCP server
attached. Key reuse on the effective allowlist instead.
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Jul 10, 2026
This was referenced Jul 10, 2026
Draft
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@macroscopeapp macroscopeapp[bot] macroscopeapp[bot] left review comments

Assignees

No one assigned

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@PixPMusic